We should also tell folks to take care when doing this that they don't change the deployment behavior of their project. I've seen folks hoist a dependency that was marked with PrivateAssets and ExcludeAssets and forget to add those same markings.

Something like?

Note: This does not change the deployment behavior in any way.

No, I mean the user needs to take care to make sure that they don't do this. So if they were previously doing:

<PackageRefence Include="Direct" Version="1.0.0" PrivateAssets="All" ExcludeAssets="Runtime" />

They should take care when adding a new reference to give it the same:

<PackageRefence Include="Direct" Version="1.0.0" PrivateAssets="All" ExcludeAssets="Runtime" />
<!-- Note: Apply the same values for PrivateAssets and ExcludeAssets as Direct --> 
<PackageRefence Include="Indirect" Version="1.1.0" PrivateAssets="All" ExcludeAssets="Runtime"/>

It's not automatic here. It is for CPM - that's a major benefit it gives.

ah i see. you're talking about the attribute behavior.

@jeffkl @zivkan should we explicitly mention this?

Maybe we say: Take care when adding the dependency to match the Assets settings of the existing dependency. When doing so it should behave in the same way as if your dependency itself was updated. This is not required when using CPM.

yep. "whne adding a explicit reference, ensure you maintain your packagereference attributes for the asset behavior" etc

I'm confused, or I think we have inconsistent behaviour (which now would be a breaking change to fix). Today, so coincidence it's on the same day, I was pinged on Discord because someone claims that a package reference with ExcludeAssets="runtime" does not exclude that package's dependencies runtime assets, and they had to manually list every transitive package as direct references and add the same ExcludeAssets="runtime".

If CPM's central pinning does flow through include/private/exclude assets, then it sounds like it changes behaviour compared to the same package not being pinned, which feel like a bug to me.

I just did a super quick test, and it looks like excludeassets does flow, so the person on discord who pinged me has a more complex scenario.

I think yes, it is worth mentioning. Include/Private/Exclude Assets might not be used by a high percentage of customers, but I would be surprised if the people using it will remember that asset group selection flows when they promote a transitive package to a direct reference (especially since none of our tools (VS, or dotnet add package) provide a way to view or edit it.

Here's a good example of the sort of thing that makes this complicated: dotnet/aspnetcore#57560 (comment)

I think we should explicitly warn people that if they pack the project into a package, the pinned package will become a package dependency. The linked doc mentions that it promotes a package to a top level dependency, but the very first two customers who tested the first CPM prototype didn't understand this, complained, and stopped using CPM.

That probably belongs in the CPM docs. Remind folks that rolling up dependencies doesn't actually add any new dependencies than what you already have. Maybe NuGet gallery could even help here by showing authored dependencies visually different than transitive ones?

nuget.org (NuGetGallery) only show dependencies listed in the nuspec, so only direct dependencies, not transitive dependencies.

Any package that CPM promotes via transitive pinning becomes a direct dependency (the page on CPM explicitly states this), and therefore it will be listed in the nuspec as a dependency. NuGet Gallery has no way to know that "this was a transitive package that was pinned by the project that it was packed from". The very first 2 customers who tried CPM (both package authors) when it was in preview and transitive pinning was default with no option to disable, immediately said they hated it and stopped using CPM. So, I think it's valuable to set expectations.

I still see transitive pinning (or direct references) as a valid way for library authors to remove transitive packages with known vulnerabilities. But if a single repo is building both packages and apps, the developer might not want to cause a transitive package like System.Text.Json to become a direct dependency of their package, when they upgrade the package for the web/gui/console apps.

There's another challenge here that the CPM docs aren't really complete in this scenario.
Separate problem: NuGet/Home#13811.

I don't know what I was thinking with my last reply. Let me try again to make it more clear:

Even if it's documented elsewhere, the first set of customers who tried transitive pinning hated it because it caused additional package dependencies, and told us they immediately stopped using it. I'm sure those two people were not unique and other customers will feel similarly. Therefore, if we're going to advise people to use the feature, I think we should set expectations so they're not surprised.

Oh yeah fair, I was just pointing out that we have another gap to close, which is document it for CPM in the first place.

Maybe we could add a link to the dotnet nuget why command or Visual Studio transitive dependencies feature documentation to identify the top-level package. I am okay with addressing this comment in a follow-up PR.

@jebriede solution explorer ships what release again?

@JonDouglas do you mean transitive dependencies in Solution-level PM UI? If so, 17.12.